Skip to content

🤖 tests: cover sendTreeMessage paths before the split - #7708

Merged
ThomasK33 merged 2 commits into
mainfrom
refactor/task-send-tests
Oct 29, 2026
Merged

ThomasK33 merged 2 commits into
mainfrom
refactor/task-send-tests

Conversation

@ThomasK33

@ThomasK33 ThomasK33 commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

Summary

Adds 6 characterization tests for TaskService.sendTreeMessage, with no production change. Line coverage of the method goes from 98 % (529 of 538 lines) to 100 %. This is the test PR at the start of stack 6 of the TaskService hotspot split. The split PRs on top of it split the method (CC 133) into named steps.

What the tests cover

The new file is taskService.sendTreeMessage.test.ts. Every test uses two root chats with a shared tag: alpha sends to beta.

Test Path
The relation changes while the send waits for the target's lock The relation check under the lock refuses with invalid_scope.
A retry after a delegated turn, with a new tag generation The retry refuses with invalid_scope.
An interrupted target chain that only the host's admission check sees The send refuses with refused, the same result as a send to a target that is already interrupted.
The target is removed before the host's admission check The send refuses with not_found.
A retry whose send fails after a new delegated turn starts The send refuses with the delegated-turn refusal, not send_failed.
A user Stop lands while the send waits for a delegated turn The park is refused with refused. Nothing parks, nothing sends, and no delivery is recorded.
  • Test 3 edits private state: it adds the target to the private interruptedParentWorkspaceIds set directly. The public interrupt also bumps the stop epoch, and the admission check reads the epoch first, so no public call reaches this backstop. A comment in the test says so.
  • Tests 3 and 4 use the host's check: they change state inside the mocked sendMessage, then run the same admission check the host runs. The checkout await has no hook that a test can use.
  • No test checks text: no test checks log or reason text.
  • Background work: each test waits for the background work it starts, including the parked-send flushes and a background config save.

Validation

  • All 6 tests pass on the current code.
  • The 6th test came from the split PRs: after the split, one branch had no test, because before the split it sat on one line with a covered if. It passes on the unsplit code and on the split code, and it fails on both when the branch is removed.
  • I made a scratch edit to each covered branch, then restored the file. Each edit failed its own test.
  • One line is redundant: the delegated-turn refusal near the end returns the same object as the line after it, so no test can tell them apart. The split keeps that line.

Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high • Cost: $198.09

Characterization tests for the five sendTreeMessage paths no test reached: the relation re-check under the target's lock, the retry tag-generation fence, the admission probe's interrupted-target and missing-target refusals, and the retry whose send fails after a new delegated turn. No production change.

Signed-off-by: Thomas Kosiewski <tk@coder.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-28T23:10:58.968283Z e87aa29 Manual request
🔒 Security Review ✅ Completed 2026-10-29T01:31:12.605735Z e87aa29 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: 1fd288ae43

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 1fd288ae43

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

🟢 e2e stories (Haiku): 14 passed

22 agent steps · 7 replayed from cache · 15 model calls · 46.5k tokens (24% cached)
Spend: $0.09 of the $1.00 cap (proxy ledger)

All 14 tests in 9 files
Test Agent Time
🟢 chatRestart.e2e.ts · 1 passed 1 step · 1 call 6.8s
🟢 a chat survives a restart of the server 1 step · 1 call 6.8s
🟢 compaction.e2e.ts · 1 passed 1 step · 1 call 4.8s
🟢 /compact replaces the old chat with a summary 1 step · 1 call 4.8s
🟢 pluginTrustRestart.e2e.ts · 1 passed 4 steps · 2 calls 10.8s
🟢 a trusted plugin hook stays trusted after the server restarts 4 steps · 2 calls 10.8s
🟢 settingsRoundTrip.e2e.ts · 1 passed 2 steps · 1 call 10.3s
🟢 a chat survives trips through Settings 2 steps · 1 call 10.3s
🟢 sidebarPerWorkspace.e2e.ts · 1 passed 4 steps · 1 call 5.5s
🟢 a new workspace starts with its right sidebar collapsed while another keeps it open 4 steps · 1 call 5.5s
🟢 smoke.e2e.ts · 1 passed 2 steps · 1 call 2.9s
🟢 the agent opens the playground workspace 2 steps · 1 call 2.9s
🟢 streamErrors.e2e.ts · 1 passed 1 step · 1 call 7.7s
🟢 stream errors show in the chat, and the chat recovers 1 step · 1 call 7.7s
🟢 todoNudge.e2e.ts · 1 passed 1 step · 1 call 4.1s
🟢 the agent finishes open TODO items after the turn-end nudge 1 step · 1 call 4.1s
🟢 touchAlignment.e2e.ts · 6 passed 6 steps · 6 calls 26.0s
🟢 the Settings close button is level with the title on a touch screen 390px wide 1 step · 1 call 5.1s
🟢 the Settings close button is level with the title on a touch screen 768px wide 1 step · 1 call 4.4s
🟢 Backup settings checkboxes are small and level with their labels on a touch phone 1 step · 1 call 5.0s
🟢 the heartbeat checkbox is small and level with its label on a touch phone 1 step · 1 call 3.5s
🟢 a reply's model and time stay on one line on a touch phone 1 step · 1 call 4.5s
🟢 the workspace actions menu stays on the title row on a touch phone 1 step · 1 call 3.6s

e2e 0.20.0 · 1m 23s · web · run artifacts

🟢 e2e Electron stories (Haiku): 3 passed

3 agent steps · 3 model calls · 7.9k tokens (22% cached)
Spend: $0.02 of the $1.00 cap (proxy ledger)

All 3 tests in 3 files
Test Agent Time
🟢 electron/deepLink.e2e.ts · 1 passed 1 step · 1 call 7.1s
🟢 a deep link of a second launch starts a chat in the running app 1 step · 1 call 7.1s
🟢 electron/menuBar.e2e.ts · 1 passed 1 step · 1 call 7.5s
🟢 the workspace menu bar is the desktop title bar 1 step · 1 call 7.5s
🟢 electron/smoke.e2e.ts · 1 passed 1 step · 1 call 6.4s
🟢 Xum Electron opens the playground workspace 1 step · 1 call 6.4s

e2e 0.20.0 · 23.7s · electron · run artifacts

🟢 e2e agent repros (Haiku): no tests selected

Spend: $0.00 of the $1.00 cap (proxy ledger)

e2e 0.20.0 · 7.1s · 2 targets (web, phone) · run artifacts

Covers the refusal in the delegated-turn wait: a first peer send that
would wait for another workspace's delegated turn is refused, not parked,
when the user stopped the target after the send was admitted. The test
asserts the refused result, that nothing is parked, sent or counted as
delivered. No production change.

---

_Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$9.55`_

<!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=9.55 -->

Signed-off-by: Thomas Kosiewski <tk@coder.com>
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: e87aa29744

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: e87aa29744

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: e87aa29744

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: e87aa29744

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: e87aa29744

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@ThomasK33
ThomasK33 added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit cd13fdd Oct 29, 2026
123 of 130 checks passed
@ThomasK33
ThomasK33 deleted the refactor/task-send-tests branch October 29, 2026 08:10
yermakoffivan pushed a commit to yermakoffivan/mux that referenced this pull request Oct 29, 2026
…oder#7709)

## Summary

Part 1 of 5: splits the tail of `TaskService.sendTreeMessage` (park,
dispatch and settle) in place into named steps, with no behaviour
change. The method drops from CC 133 to CC 109. On top of the test PR
coder#7708. 460 changed lines.

The parts go from the end of the method to its start, as in the two
earlier stacks (coder#7697 to coder#7699, coder#7702 to coder#7706). The code that stays
inline must be the start of the method, so it can call the extracted
tail with one call.

## Steps in this part

- `dispatchPeerSend` makes the dispatch.
- `refuseStalePeerSendBeforeDispatch` runs the recheck before dispatch.
- `queuePeerSendBehindParked` queues the send behind a parked one.
- `parkPeerSendForDelegatedTurn` is the former `park` closure, and
`waitForDelegatedTurn` waits for the delegated turn.
- `settlePeerSendResult` keeps the refusal order: the delegated-turn
refusal, then the admission refusal, then `send_failed`.
- This part adds the readonly records `PeerSendRequest`, `PeerSendRoute`
and `PeerSendAdmission`, and the mutable `PeerSendAttempt`. In this part
the admission check is still a closure, and `dispatchPeerSend` gets it
as a parameter.

## Rules every part keeps

- The four overloads and the implementation signature of
`sendTreeMessage` do not change. No step calls `this.sendTreeMessage`
again, so the spy in `taskService.delegatedTurnPeerDelivery.test.ts`
still sees every call.
- Every step runs inside `withPeerAdmissionLock(targetId, ...)`, which
is the peer delivery lock plus the target's event lock. No step takes
either lock, and the callback returns the chain, so both locks release
when the chain settles.
- The callbacks that `sendMessage` and the workspace service run later
(`admissionStale`, park, `onCanceled`, `onAccepted`) read and write one
mutable `PeerSendAttempt` record when they run:
`delegatedTurnCorrelation`, `admissionRefusal`,
`refusedAtRegistrationEpoch`, `parkedThisAttempt` and `accepted`.
`admissionStale` stays a plain synchronous function.
- The steps form a chain: each step calls the next one directly after
its own last await, so no collaborator call runs a microtask later. Only
the result of the send settles later, at approximately 2 to 3 microtasks
per chain level. So the two locks also release that much later. A
refusal before the checkout await settles at the same time as before.
- `make crap-check` against the parent commit reports 0 violations, with
and without coverage. Every new function scores CC 8 or less.
- A script compared the old and new code: both have 5 awaits and the
same collaborator calls.

---

_Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking:
`high` • Cost: `$198.09`_

<!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high
costs=198.09 -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant